-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
switch to ruff #5218
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
switch to ruff #5218
Conversation
@gvwilson big picture comments --
|
af41d30
to
9c60129
Compare
CONTRIBUTING.md
Outdated
## Have Questions about Plotly? | ||
We use Git and GitHub to manage our project; | ||
if you are not familiar with them, | ||
there are great resources like <http://try.github.io/> to get your started. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are great resources like <http://try.github.io/> to get your started. | |
there are great resources like <http://try.github.io/> to get you started. |
CONTRIBUTING.md
Outdated
|
||
```bash | ||
$ python commands.py updateplotlyjs | ||
pytest tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pytest tests | |
python -m pytest tests |
CONTRIBUTING.md
Outdated
$ npm pack | ||
$ mv plotly.js-*.tgz plotly.js.tgz | ||
To test `go.FigureWidget` locally, you'll need to generate the JavaScript bundle as follows: | ||
|
||
# In your plotly.py/ directory: | ||
$ python commands.py updateplotlyjsdev --local /path/to/your/plotly.js/ | ||
``` | ||
cd js | ||
npm install && npm run build | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what I should be deleting here - can you please re-check after I push an update and let me know?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah sorry that was ambiguous. I'm suggesting to delete lines 179-184 on your branch in CONTRIBUTING.md. The following lines convey the same information and are more clear, I think.
What would you think of adding an item to the pull request template documentations section that recommends: [ ] start Python code blocks with ```python? It's something that is not obvious in the checklist and that I've forgotten to do a couple times. Forgetting to do so can prevent the example from displaying. |
- Remove `requires-optional.txt` and `test_requirements/*` (no longer used). - Convert `commands.py` to use `argparse` instead of hand-rolled. - Rationalize imports in `codegen/__init__.py` (all at top level). - Update `"dev"` section of `pyproject.toml` to use `ruff` instead of `black`. - Update `pyproject.toml` to install `inflect` and `requests`. - Modify `uv.lock` file. - Modify code reformatting in `codegen/__init__.py` to use `ruff` instead of `black`. - Add new function to use `ruff` to reformat existing code (run `python commands.py format`). - Add new function to use `ruff` to check code (run `python commands.py lint`). This PR currently fails the build because Circle CI is expecting `requires-optional.txt`, which this PR removes. We need to decide if we're leaving build on Circle CI or moving it to GitHub.
A previous PR in this branch removed `test_requirements/requirements_core.txt` and `test_requirements/requirements_optional.txt`. @emilylk pointed out that these files are used in Circle CI to set up a minimal environment for running `tests/test_core/*` and then a larger environment for running the other tests (the idea being to prevent extra dependencies from sneaking into plotly.py). This PR divides dependencies into `dev_core` and `dev_optional` with `dev` essentially aliasing `dev_optional` so that `./.circleci/config.yml` can be modified to use these for installation. Note: `uv.lock` reflects `dev_optional`, i.e., pins *all* dependencies.
@gvwilson Add this line to the
|
- Modify `pyproject.toml` to have a separate `dev_build` section with packages needed for building the plotly package. - Replace uses of `uv pip` with `uv sync` in `.circleci/config.yml`.
e908dad
to
6ce94b2
Compare
- Modify `pyproject.toml` to exclude `plotly/graph_objs` from checking for now. - Modify `pyproject.toml` to include `anywidget` in core dependencies (because `plotly/basewidget.py` needs it). - Clean up several hundred complaints from `ruff` about unused variables and imports. - Clean up cases in `tests/**/*.py` where the same function name was used for several tests (which meant that only the last one defined was actually run). - Reformat code after recent changes.
requires-optional.txt
andtest_requirements/*
(no longer used).commands.py
to useargparse
instead of hand-rolled.codegen/__init__.py
(all at top level)."dev"
section ofpyproject.toml
to useruff
instead ofblack
.pyproject.toml
to installinflect
andrequests
.uv.lock
file.codegen/__init__.py
to useruff
instead ofblack
.ruff
to reformat existing code (runpython commands.py format
).ruff
to check code (runpython commands.py lint
).This PR has been rebuilt on top of #5214 to include recent changes to validator generation.
This PR currently fails the build because Circle CI is expecting
requires-optional.txt
, which this PR removes.We need to decide if we're leaving build on Circle CI or moving it to GitHub.